Add getEtchPacket and downloadDocuments Methods #26

Winggo · 2020-10-01T23:56:43Z

Description of the change

Having the getEtchPacket method in the client would be extremely helpful for API users to be able to get their etch packet details. Because currently the only time API users are able to get etch packet details is right after they have created it. Also needed for building the signatures API example.

The downloadDocuments method hits our /api/document-group/:documentGroupEid.zip route specifically for client API Zip document downloads. Gives an easy abstracted solution for users to download their documents.

getEtchPacket use cases:

get packet data for the /packet/:packetEid route in example
etchPacket documentGroup status checks

downloadDocuments use cases:

download zip documents button at end of signature workflow
user doesn't have to deal with finding the downloadZipURL, the client.downloadDocuments() method takes care of that

Added:

get-etch-packet.js example script
download-documents.js example script
docs

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Checklists

Development

The code changed/added as part of this pull request has been covered with tests
All tests related to the changed code pass in development
No previous tests unrelated to the changed code fail in development

Code review

This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached.
At least one reviewer has been requested
Changes have been reviewed by at least one other engineer
The relevant project board has been selected in Projects to auto-link to this pull request

README.md

src/index.js

src/graphql/queries/etchPacket.js

src/index.js

…teEtchSignUrlMutation

src/graphql/mutations/createEtchPacket.js

newhouse

Overall looks sweet! Had some minor comments in the code that don't necessarily require anything to happen before I would approve.

But, there are no tests. Can you add some simple tests for these? There are some patterns for each already in the /tests directory, and I'm happy to help you with them, too.

newhouse · 2020-10-14T18:22:44Z

example/script/download-documents.js

+      data.on('error', reject)
+      writeStream.on('finish', resolve)
+    })
+    console.log(statusCode, data)


We sure we want to write this data to the console? It would be a lot of blah blah blah, no?

And below, too?

It'd be a PassThrough stream object, but I agree it's not necessary. I'll take that out.

newhouse · 2020-10-14T18:26:59Z

src/graphql/mutations/createEtchPacket.js

@@ -3,24 +3,28 @@ const defaultResponseQuery = `{
  id
  eid
  name
+  detailsURL


Let's be mindful of what's added here that may be to satisfy your example repo usage vs. what should truly be sensible defaults. If there's something you think might not reasonably belong in a default response query then let's leave it out...and if your app needs it, then it can pass its own custom response query.

I think that these are all fine in that regard, and no need to change them, but let's be mindful.

Agreed and thanks for pointing that out. I'm taking downloadZipURL out as I don't see it being necessary.

newhouse · 2020-10-14T18:30:04Z

src/index.js

+    return this.requestREST(
+      `/api/document-group/${documentGroupEid}.zip`,
+      { method: 'GET' },
+      { dataType: DATA_TYPE_STREAM })


Why default to Stream here vs Buffer (which is the default for fillPDF? And why not allow it to be overridden to be the other type (Buffer in this case)?

Defaulted to stream since it was how I used the API in the example repo. But I've just added the option to choose between stream and buffer, following the approach of fillPDF with buffer being the default.

newhouse

Just 1 small comment, and some tests and this baby is ready to go. Will approve to keep from slowing you down.

Nice!

newhouse · 2020-10-15T15:23:32Z

README.md

+* Returns a `Promise` that resolves to an `Object`
+   * `statusCode` (Number) - the HTTP status code, `200` is success
+   * `response` (Object) - the Response object resulting from the client's request to the Anvil app
+   * `data` (object) - a PassThrough Stream object containing the document group's data in Zip file format


Should update this to be more in-line with what's now actually going to happen: buffer of stream. Probably worth following the pattern of fillPDF and/or updating both.

Will update!

newhouse

super minor comments and then just merge it!

newhouse · 2020-10-15T22:13:29Z

README.md

+* Returns a `Promise` that resolves to an `Object`
+   * `statusCode` (Number) - the HTTP status code, `200` is success
+   * `response` (Object) - the Response object resulting from the client's request to the Anvil app
+   * `data` (Buffer | Stream) - The raw binary data of the downloaded document if success. Will be in the format of either a Buffer or a Stream, depending on `dataType` option supplied to the request.


Suggested change

* `data` (Buffer | Stream) - The raw binary data of the downloaded document if success. Will be in the format of either a Buffer or a Stream, depending on `dataType` option supplied to the request.

* `data` (Buffer | Stream) - The raw binary data of the downloaded documents if success. Will be in the format of either a Buffer or a Stream, depending on `dataType` option supplied to the request.

Maybe even refer to the fact that they're zipped?

newhouse · 2020-10-15T22:14:54Z

test/index.test.js

+      })
+
+      context('unsupported options', function () {
+        it('returns data as buffer', async function () {


Suggested change

it('returns data as buffer', async function () {

it('raises appropriate error', async function () {

add getEtchPacket query

5f43011

Winggo requested review from benogle and newhouse October 1, 2020 23:56

newhouse reviewed Oct 2, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

src/graphql/queries/etchPacket.js Outdated Show resolved Hide resolved

review changes

225f44a

newhouse reviewed Oct 2, 2020

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

Winggo added 2 commits October 2, 2020 13:09

add providerConfig to getEtchPacket to access org slug, rename genera…

cc7aedd

…teEtchSignUrlMutation

add signer status in defaultResponseQuery

a5220e4

newhouse reviewed Oct 2, 2020

View reviewed changes

src/graphql/mutations/createEtchPacket.js Outdated Show resolved Hide resolved

Winggo added 4 commits October 6, 2020 11:03

remove providerConfig from defaultResponseQueries

cd6e6e2

add etchPacketDetailsURL resolver

03ea442

add downloadZipURL resolver to defaultResponseQuery

176a0d4

rename detailsURL

368b0aa

Winggo requested a review from newhouse October 6, 2020 21:36

Winggo added 4 commits October 7, 2020 16:16

add signActionType to defaultResponseQuery

2c76dc4

add downloadDocumentGroup method

c11e046

fix downloadDocuments method

507d622

downloadDocuments docs

33809da

Winggo changed the title ~~Add getEtchPacket Query~~ Add getEtchPacket and downloadDocuments Methods Oct 9, 2020

Winggo added 3 commits October 9, 2020 16:42

update downloadDocuments route

6326dd3

update route

7c6a970

some tiny text fixes

7d6e00e

newhouse reviewed Oct 14, 2020

View reviewed changes

Winggo added 2 commits October 14, 2020 15:07

add options to downloadDocuments

760ea4b

add -s flag in downloadDocuments usage

34e3a67

newhouse approved these changes Oct 15, 2020

View reviewed changes

tests for downloadDocuments, generateEtchSignUrl, getEtchPacket

dec02fb

Winggo self-assigned this Oct 15, 2020

newhouse approved these changes Oct 15, 2020

View reviewed changes

minor fixes

7384c0a

Winggo merged commit f12ea90 into master Oct 15, 2020

Winggo deleted the wt/getEtchPacket branch October 15, 2020 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add getEtchPacket and downloadDocuments Methods #26

Add getEtchPacket and downloadDocuments Methods #26

Winggo commented Oct 1, 2020 •

edited

Loading

newhouse left a comment

newhouse Oct 14, 2020

Winggo Oct 14, 2020

newhouse Oct 14, 2020

Winggo Oct 14, 2020

newhouse Oct 14, 2020

Winggo Oct 14, 2020

newhouse left a comment

newhouse Oct 15, 2020

Winggo Oct 15, 2020

newhouse left a comment

newhouse Oct 15, 2020

newhouse Oct 15, 2020

newhouse Oct 15, 2020

	* `data` (Buffer \| Stream) - The raw binary data of the downloaded document if success. Will be in the format of either a Buffer or a Stream, depending on `dataType` option supplied to the request.
	* `data` (Buffer \| Stream) - The raw binary data of the downloaded documents if success. Will be in the format of either a Buffer or a Stream, depending on `dataType` option supplied to the request.

	it('returns data as buffer', async function () {
	it('raises appropriate error', async function () {

Add getEtchPacket and downloadDocuments Methods #26

Add getEtchPacket and downloadDocuments Methods #26

Conversation

Winggo commented Oct 1, 2020 • edited Loading

Description of the change

Type of change

Related issues

Checklists

Development

Code review

newhouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

newhouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

newhouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Winggo commented Oct 1, 2020 •

edited

Loading